refactor: remove request_ctx ContextVar, thread Context explicitly#2203
refactor: remove request_ctx ContextVar, thread Context explicitly#2203
Conversation
The request_ctx ContextVar in mcp.server.lowlevel.server was redundant with the ServerRequestContext already passed as the first argument to every _handle_* method. This removes the ContextVar entirely and threads Context explicitly. Changes: - MCPServer.get_context() removed — use ctx: Context parameter injection in tool/resource/prompt functions instead - MCPServer.call_tool/read_resource/get_prompt now accept an optional context: Context | None = None parameter; _handle_* methods construct the Context at the lowlevel boundary and pass it through - Context class moved from server.py to its own context.py module (still re-exported from mcp.server.mcpserver) - Tool.run/Prompt.render/ResourceTemplate.create_resource now raise a clear error if the registered function requires a Context but none was provided, instead of silently injecting None Github-Issue: #2112 Github-Issue: #1684
maxisbey
requested review from
Kludex and
felixweinberger
and removed request for
Kludex
| if self.context_kwarg is not None and context is None: | ||
| raise ValueError(f"Prompt {self.name!r} requires a Context, but none was provided") |
There was a problem hiding this comment.
How would the Context be None here? 🤔
There was a problem hiding this comment.
Also, probably it's RuntimeError, if anything.
There was a problem hiding this comment.
Honestly the way Context is optional is rather weird through the whole call chain. If it's None it's sometimes just not injected which also breaks stuff.
After taking a bit of time to walk through it, I think it'd be better to leave it as optional on the MCPServer methods itself, but then make it required on the rest of the call stack, which is essentially the same functionality that existed before. If you called the previous MCPServer.get_context method it would just construct a new one if none existed.
So will do that to restore what was here before and remove some of the weird optional handling through the rest of the call chain.
| if self.context_kwarg is not None and context is None: | ||
| raise ValueError(f"Resource template {self.name!r} requires a Context, but none was provided") |
| if self.context_kwarg is not None and context is None: | ||
| raise ToolError(f"Tool {self.name!r} requires a Context, but none was provided") |
| from mcp.server.elicitation import ( | ||
| elicit_url as _elicit_url, | ||
| ) |
There was a problem hiding this comment.
What's this?
| from mcp.server.elicitation import ( | |
| elicit_url as _elicit_url, | |
| ) | |
| from mcp.server.elicitation import elicit_url |
There was a problem hiding this comment.
Ah, got it.
| from mcp.server.elicitation import ( | |
| elicit_url as _elicit_url, | |
| ) | |
| from mcp.server.elicitation import elicit_url as _elicit_url |
Still...
| # TODO(Marcelo): We should drop this kwargs parameter. | ||
| **kwargs: Any, |
There was a problem hiding this comment.
I think we should follow what Marcelo said.
There was a problem hiding this comment.
Ah I wasn't actually changing the Context class at all, just moving it for now
| """ | ||
| progress_token = self.request_context.meta.get("progress_token") if self.request_context.meta else None | ||
|
|
||
| if progress_token is None: # pragma: no cover |
There was a problem hiding this comment.
| if progress_token is None: # pragma: no cover | |
| if progress_token is None: # pragma: no branch |
| related_request_id=self.request_id, | ||
| ) | ||
|
|
||
| async def read_resource(self, uri: str | AnyUrl) -> Iterable[ReadResourceContents]: |
There was a problem hiding this comment.
Do we need this AnyUrl?
| async def read_resource(self, uri: str | AnyUrl) -> Iterable[ReadResourceContents]: | |
| async def read_resource(self, uri: str) -> Iterable[ReadResourceContents]: |
There was a problem hiding this comment.
keen to follow up with a different PR to actually refactor the Context class itself, but this one is just about moving it out and removing the context_var access.
| async def close_sse_stream(self) -> None: | ||
| """Close the SSE stream to trigger client reconnection. | ||
|
|
||
| This method closes the HTTP connection for the current request, triggering | ||
| client reconnection. Events continue to be stored in the event store and will | ||
| be replayed when the client reconnects with Last-Event-ID. | ||
|
|
||
| Use this to implement polling behavior during long-running operations - | ||
| the client will reconnect after the retry interval specified in the priming event. | ||
|
|
||
| Note: | ||
| This is a no-op if not using StreamableHTTP transport with event_store. | ||
| The callback is only available when event_store is configured. | ||
| """ | ||
| if self._request_context and self._request_context.close_sse_stream: # pragma: no cover | ||
| await self._request_context.close_sse_stream() | ||
|
|
||
| async def close_standalone_sse_stream(self) -> None: | ||
| """Close the standalone GET SSE stream to trigger client reconnection. | ||
|
|
||
| This method closes the HTTP connection for the standalone GET stream used | ||
| for unsolicited server-to-client notifications. The client SHOULD reconnect | ||
| with Last-Event-ID to resume receiving notifications. | ||
|
|
||
| Note: | ||
| This is a no-op if not using StreamableHTTP transport with event_store. | ||
| Currently, client reconnection for standalone GET streams is NOT | ||
| implemented - this is a known gap. | ||
| """ | ||
| if self._request_context and self._request_context.close_standalone_sse_stream: # pragma: no cover | ||
| await self._request_context.close_standalone_sse_stream() |
There was a problem hiding this comment.
Did we find out the answers for what I asked before? When would the developer actually use those???
There was a problem hiding this comment.
iirc I couldn't find actual uses for it publically, but it's a required part of the spec the sdk needs to support
There was a problem hiding this comment.
This doesn't seem a reasonable answer. So we have a public API that is required by a spec that actually no one uses? Is the spec written for the sake of having a spec? Can we please find the use case of it?
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
2 participants
Removes the
request_ctxContextVar and threadsContextexplicitly through the MCPServer request-handling chain.Motivation and Context
The
request_ctxContextVar inmcp.server.lowlevel.serverwas redundant: the lowlevel server already passesServerRequestContextas the first argument to every_handle_*method. The ContextVar was a second mechanism carrying the same value, used only byMCPServer.get_context().This PR removes the ContextVar entirely, making the data flow explicit.
_handle_*methods construct the high-levelContextat the lowlevel→MCPServer boundary and pass it through.Part of #2112 (context refactor) / #1684 (contextvars cleanup).
How Has This Been Tested?
Tool.run(),Prompt.render(),ResourceTemplate.create_resource()uv run --frozen pyright src/ tests/— 0 errorsuv run --frozen ruff check— cleanBreaking Changes
Yes — documented in
docs/migration.md:MCPServer.get_context()removed — usectx: Contextparameter injection in tool/resource/prompt functions instead (the existing recommended pattern)request_ctxContextVar removed frommcp.server.lowlevel.server— code importing it directly will need to use parameter injectionMCPServer.call_tool(),read_resource(),get_prompt()now accept an optionalcontext: Context | None = Noneparameter — backward-compatible for callers that don't pass itctx: Contextparameter but is called withcontext=None, a clear error is raised (ToolErrorfor tools,ValueErrorfor prompts/resource templates). PreviouslyNonewas silently injected.Types of changes
Checklist
Additional context
Contextclass moved frommcp.server.mcpserver.servertomcp.server.mcpserver.context. The public import path (from mcp.server.mcpserver import Context) is unchanged via__init__.pyre-export. The old direct-module path (from mcp.server.mcpserver.server import Context) also still works becauseserver.pyimportsContextat module level.Exception hierarchy cleanup (making prompts/resources raise
MCPServerErrorsubclasses instead ofValueError) was considered but deferred to #1742 (error taxonomy) to avoid scope creep — see that issue for the full analysis of the existing inconsistencies.AI Disclaimer